Skip to content

fix(a11y) Make Dropdown more accessible #2333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 13, 2018

Conversation

tryzniak
Copy link
Contributor

@tryzniak tryzniak commented Jul 4, 2018

  • make it focusable
  • allow navigation using arrow keys
  • dismiss on escape

This is a rework of #2325

@@ -45,8 +45,8 @@ export default class Navigation extends React.Component {
<Dropdown
className="navigation__languages"
items={[
{ title: 'English', url: 'https://webpack.js.org/' },
{ title: '中文', url: 'https://doc.webpack-china.org/' }
{ lang: 'en', title: 'English', url: 'https://webpack.js.org/' },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason of this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's my mistake. This shouldn't be there. Thanks for taking a look :)

- make it focusable
- allow navigation using arrow keys
- dismiss on escape
@tryzniak tryzniak force-pushed the fix/a11y-dropdown-rebuild branch from a0626b0 to b630674 Compare July 5, 2018 14:04
@@ -46,7 +46,7 @@ export default class Navigation extends React.Component {
className="navigation__languages"
items={[
{ title: 'English', url: 'https://webpack.js.org/' },
{ title: '中文', url: 'https://doc.webpack-china.org/' }
{ lang: 'zh', title: '中文', url: 'https://doc.webpack-china.org/' }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fine to left English language with an empty lang attribute?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, because we have already set lang globally with , so we don't have to set it here.

@montogeek montogeek merged commit 8bb23dd into webpack:rebuild Jul 13, 2018
@montogeek
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants